Skip to content

Conversation

@dang
Copy link
Contributor

@dang dang commented Apr 8, 2025

We clean up buckets 3 times, using each client, so that all buckets can be removed, regardless of which client created it. However, the cleanup will throw an exception if permission is denied, due to using the wrong
user. This will stop cleanup, and cause all subsequent tests to ERROR.
Instead, pass these exceptions, allowing full cleanup.

We clean up buckets 3 times, using each client, so that all buckets can
be removed, regardless of which client created it.  However, the cleanup
will throw an exception if permission is denied, due to using the wrong
user.   This will stop cleanup, and cause all subsequent tests to ERROR.
Instead, pass these exceptions, allowing full cleanup.

Signed-off-by: Daniel Gryniewicz <[email protected]>
@dang dang requested a review from cbodley April 8, 2025 16:41
@cbodley
Copy link
Contributor

cbodley commented Apr 8, 2025

i guess i'm conflicted on this. usually, i only see cleanup permission errors where something in the test case failed, so didn't run the logic to restore permissions. i view this as a bug in the test case itself. #428 is an example where we added try/finally for unconditional cleanup

if the cleanup logic ignores all errors, we risk introducing new test cases that silently leave things behind. i think i'd prefer to leave those as failures

@dang
Copy link
Contributor Author

dang commented Apr 11, 2025

The test case I'm trying to fix is test_object_raw_get_x_amz_expires_not_expired_tenant. It creates a bucket with "public-read" and an object with "public-read" using the tenant_client. Then, teardown tries (first) do delete with the normal client, and fails, and aborts. I don't see how this test can ever pass.

@dang
Copy link
Contributor Author

dang commented Apr 11, 2025

The alternative would be to not ERROR test cases unless that test case left things behind. The issue is that all the rest of the test cases ERROR for the rest of the run.

@cbodley
Copy link
Contributor

cbodley commented Apr 14, 2025

The test case I'm trying to fix is test_object_raw_get_x_amz_expires_not_expired_tenant. It creates a bucket with "public-read" and an object with "public-read" using the tenant_client. Then, teardown tries (first) do delete with the normal client, and fails, and aborts. I don't see how this test can ever pass.

why does the normal client see the tenanted user's bucket? it uses client.list_buckets() which should only return buckets owned by that user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants